Skip to content

Refactoring: Move flash messages from Task services to Controller actions (Case 212955) - #29

Merged
janopae merged 3 commits into
masterfrom
212955-refactor-session-usage-out-of-tasks
Jul 31, 2026
Merged

Refactoring: Move flash messages from Task services to Controller actions (Case 212955)#29
janopae merged 3 commits into
masterfrom
212955-refactor-session-usage-out-of-tasks

Conversation

@janopae

@janopae janopae commented Jul 31, 2026

Copy link
Copy Markdown
Member

The Task services currently inject RequestStack and assert-cast the session to FlashBagAwareSessionInterface in order to write flash messages — a workaround needed because FlashBagInterface is no longer a DI service in Symfony 7. Flash messages are a presentation concern and belong in the Controller, where Symfony's argument resolver can inject FlashBagAwareSessionInterface directly as an action parameter. Move the three flashBag->add() calls to confirmRegistration, editRegistration, and deleteRegistration in Controller.php, and remove the RequestStack dependency from the three Task classes.

@janopae janopae changed the title Refactoring: Move flash messages from Task services to Controller act… Refactoring: Move flash messages from Task services to Controller actions (Case 212955) Jul 31, 2026
janopae added 2 commits July 31, 2026 11:28
…ions

The Task services currently inject RequestStack and assert-cast the session
to FlashBagAwareSessionInterface in order to write flash messages — a
workaround needed because FlashBagInterface is no longer a DI service in
Symfony 7. Flash messages are a presentation concern and belong in the
Controller, where Symfony's argument resolver can inject
FlashBagAwareSessionInterface directly as an action parameter. Move the
three flashBag->add() calls to confirmRegistration, editRegistration, and
deleteRegistration in Controller.php, and remove the RequestStack
dependency from the three Task classes.
…tests

The Task services no longer set flash messages — that responsibility moved to
the Controller. The writes_success_flash unit tests were removed from the
three TaskTests, leaving the flash behavior untested.

Add three functional tests to ControllerTest:

- confirm_registration_sets_success_flash: creates a PendingOptIn, follows
  the confirm URL, follows the redirect to the edit page, and asserts a
  .flash-success element is rendered.
- edit_registration_sets_success_flash: submits the edit form via the
  crawler and asserts the flash appears in the re-rendered page.
- delete_registration_sets_success_flash: posts to the delete URL, follows
  the redirect to the start page, and asserts the flash appears there.

Two supporting additions were needed:

- DummyRecipientFactory: the confirm flow calls RecipientFactory, which uses
  DetermineAppsSubclassHelper to find a RecipientInterface implementation
  outside the bundle namespace. No such class is declared in the test kernel
  (Dummy\Recipient is excluded because its namespace starts with
  Webfactory\NewsletterRegistrationBundle). DummyRecipientFactory bypasses
  the scan and creates Dummy\Recipient directly, which is the Doctrine-mapped
  entity the test kernel uses. Wired via functional_services.php.
- NewsletterFactory::createMany(2) in the edit test: the newsletters form
  field — and with it the submit button — is only added to the form when at
  least two visible newsletters exist in the database.
@janopae
janopae force-pushed the 212955-refactor-session-usage-out-of-tasks branch from f9b3ff3 to 4376e17 Compare July 31, 2026 09:30
The editRegistration action picks one of two translation keys depending on
whether the recipient ends up subscribed to at least one newsletter after the
form is submitted. The existing test only covered the "no newsletters chosen"
branch. Add a second test that submits the form with one newsletter selected
and asserts the "updated" flash. Rename the existing test from
edit_registration_sets_success_flash to
edit_registration_with_no_newsletter_selected_sets_success_flash so the two
tests read as a matched pair.
@janopae
janopae merged commit 5f70cbf into master Jul 31, 2026
3 checks passed
@janopae
janopae deleted the 212955-refactor-session-usage-out-of-tasks branch July 31, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant